Skip to content

refactor: remove duplicate git diff logic in frontmatter validator#473

Merged
katriendg merged 8 commits intomicrosoft:mainfrom
cnaples79:refactor/remove-duplicate-git-diff-322
Feb 23, 2026
Merged

refactor: remove duplicate git diff logic in frontmatter validator#473
katriendg merged 8 commits intomicrosoft:mainfrom
cnaples79:refactor/remove-duplicate-git-diff-322

Conversation

@cnaples79
Copy link
Contributor

@cnaples79 cnaples79 commented Feb 11, 2026

Pull Request

Description

Removes duplicate Get-ChangedMarkdownFileGroup function from the frontmatter validator, replacing it with the shared Get-ChangedFilesFromGit helper from LintingHelpers.psm1. This eliminates 491 lines of duplicate code.

  • Removed 104-line duplicate function from Validate-MarkdownFrontmatter.ps1, replaced with shared helper call
  • Removed 386 lines of obsolete test code for the deleted function
  • Updated integration test mocks to use Get-ChangedFilesFromGit

Related Issue(s)

Fixes #322

Type of Change

Select all that apply:

Code & Documentation:

  • Bug fix (non-breaking change fixing an issue)
  • New feature (non-breaking change adding functionality)
  • Breaking change (fix or feature causing existing functionality to change)
  • Documentation update

Infrastructure & Configuration:

  • GitHub Actions workflow
  • Linting configuration (markdown, PowerShell, etc.)
  • Security configuration
  • DevContainer configuration
  • Dependency update

AI Artifacts:

  • Reviewed contribution with prompt-builder agent and addressed all feedback
  • Copilot instructions (.github/instructions/*.instructions.md)
  • Copilot prompt (.github/prompts/*.prompt.md)
  • Copilot agent (.github/agents/*.agent.md)
  • Copilot skill (.github/skills/*/SKILL.md)

Other:

  • Script/automation (.ps1, .sh, .py)
  • Other (please describe):

Testing

  • All CI checks passing
  • PowerShell tests pass
  • Shared function already has test coverage in LintingHelpers.Tests.ps1

Checklist

Required Checks

  • Documentation is updated (if applicable)
  • Files follow existing naming conventions
  • Changes are backwards compatible (if applicable)
  • Tests added for new functionality (if applicable)

Required Automated Checks

The following validation commands must pass before merging:

  • Markdown linting: npm run lint:md
  • Spell checking: npm run spell-check
  • Frontmatter validation: npm run lint:frontmatter
  • Skill structure validation: npm run validate:skills
  • Link validation: npm run lint:md-links
  • PowerShell analysis: npm run lint:ps

Security Considerations

  • This PR does not contain any sensitive or NDA information
  • Any new dependencies have been reviewed for security issues
  • Security-related scripts follow the principle of least privilege

Additional Notes

Files changed:

  • scripts/linting/Validate-MarkdownFrontmatter.ps1 (-106 lines)
  • scripts/tests/linting/Validate-MarkdownFrontmatter.Tests.ps1 (-386 lines)

- Remove Get-ChangedMarkdownFileGroup test region (function deleted)
- Remove Git Fallback Strategy tests (FallbackStrategy param not in shared function)
- Remove ChangedFilesOnly Integration tests (function no longer exists)
- Update integration test mocks to use Get-ChangedFilesFromGit
- Update parameter filters to include FileExtensions check
- Update test names to reference correct function

Fixes failing PowerShell tests after refactoring to use shared helper
@cnaples79 cnaples79 requested a review from a team as a code owner February 11, 2026 00:44
@codecov-commenter
Copy link

codecov-commenter commented Feb 11, 2026

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 84.51%. Comparing base (975e862) to head (9fbbac2).

Files with missing lines Patch % Lines
scripts/linting/Validate-MarkdownFrontmatter.ps1 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #473      +/-   ##
==========================================
+ Coverage   84.24%   84.51%   +0.26%     
==========================================
  Files          24       24              
  Lines        4893     4868      -25     
==========================================
- Hits         4122     4114       -8     
+ Misses        771      754      -17     
Flag Coverage Δ
pester 84.51% <0.00%> (+0.26%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
scripts/linting/Validate-MarkdownFrontmatter.ps1 78.59% <0.00%> (+3.93%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@katriendg katriendg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @cnaples79 for taking on this issue and contributing improvements.

With the current approach and deleted tests however, there are a few elements no longer tested in as much detail because LintingHelpers.Tests.ps1 does not cover some of what the deleted paths tested.

Could you give it another pass using either Task-Researcher or RPI to help you question which parts of the branch have removed tests that are now no longer/not yet covered, and should now be extended in the LintingHelpers.Tests.ps1.
This will ensure we don't lose any of the granularity of the testing which was previously there.

@WilliamBerryiii
Copy link
Member

Thank you for your contribution, @cnaples79. We restructured the PR description to follow the project's pull request template. Please review the updated description and check any additional boxes that apply to your changes.

@WilliamBerryiii
Copy link
Member

Hey @cnaples79 👋

Thank you so much for this contribution! Removing that duplicate Get-ChangedMarkdownFileGroup logic and consolidating onto the shared helper was a great refactor — eliminating ~490 lines of redundant code is a solid win for maintainability.

We've finished out the remaining work on this PR to get it across the finish line. We really appreciate you taking the time to identify the duplication and put this together.

You're always welcome to come back and contribute again — we'd love to see more from you. Thanks again! 🎉

Copy link
Contributor

@katriendg katriendg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @cnaples79 and then @WilliamBerryiii for the added refactoring and PR work. This looks good now, we will get this merged.

@katriendg katriendg merged commit 69caa84 into microsoft:main Feb 23, 2026
19 checks passed
WilliamBerryiii pushed a commit that referenced this pull request Feb 28, 2026
## Pre-Release 3.1.44

### ✨ Features

- add Docusaurus 3 documentation site with GitHub Pages deployment
(#680)
- add workflow permissions validation for OpenSSF Scorecard compliance
(#759)
- add DT coach return path handoff to task-researcher (#591) (#758)
- add DT subagent handoff workflow instructions (#592) (#757)
- create dt-method-06-deep.instructions.md (#602) (#748)
- create dt-method-05-deep.instructions.md (#747)
- add DT-aware task-implementor context instructions (#755)
- extract embedded PowerShell from workflows into testable scripts
(#738)
- add gitleaks binary-based secret scanning as PR gate (#734)
- add SBOM generation, attestation, and diff tooling to release pipeline
(#730)
- add dt-learning-tutor agent for DT education (#662)
- add DT image prompt generation guidance for Method 5 (#726)
- add DT-aware task-reviewer review context (#714)
- add dt-method-next routing prompt (#713)
- create dt-method-04-deep.instructions.md (#709)
- add Implementation Space exit handoff prompt for DT workflows (#708)
- add Write-CIStepSummary markdown table to Test-SHAStaleness github
output (#660)
- add dt-handoff-solution-space prompt for Solution Spac… (#707)

### 🐛 Bug Fixes

- update sidebar link color to meet WCAG AA contrast requirements (#814)
- harden even/odd versioning against regression and syntax errors (#816)
- replace even/odd versioning with SemVer -rc.N suffixes (#811)
- ensure prerelease label exists before PR creation (#806)
- replace Docusaurus favicons with Microsoft logo (#808)
- add missing subagents and shared instructions to collection manifests
(#804)
- standardize file path conventions for copilot-tracking output (#784)
- enforce project-scoped artifact isolation across DT files (#766)
- add top-level permissions to copilot-setup-steps.yml (#760)
- update broken file directives and markdown links after collection
directory reorg (#743)
- add pre-release companion pipeline with even/odd versioning (#735)
- exclude auto-generated CHANGELOG.md from spell check (#756)
- add job-level permissions to extension-publish.yml (#729)
- resolve handoff dependencies using display names (#727)
- add job-level permissions to validate-version in
extension-publish-prerelease (#731)
- replace parent-directory VS Code settings paths with per-subdirectory
enumeration (#732)

### 📚 Documentation

- add Design Thinking documentation and DT-to-RPI handoff (#789)
- add customization guides for HVE Core artifacts (#772)
- reconcile documentation against implementation (#771)
- document accepted Token-Permissions risks and add
lint:dependency-pinning (#763)
- add Design Thinking section to hve-core-all collection description
(#762)

### ♻️ Refactoring

- move collection scripts from plugins to collections (#728)
- remove duplicate git diff logic in frontmatter validator (#473)

### 🔧 Maintenance

- bump basic-ftp from 5.0.5 to 5.2.0 (#780)
- standardize script path references in SKILL.md files (#768)
- bump the github-actions group across 1 directory with 2 updates (#752)

---
*Managed automatically by pre-release workflow.*

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
WilliamBerryiii pushed a commit that referenced this pull request Feb 28, 2026
## Pre-Release 3.1.46

### ✨ Features

- add Docusaurus 3 documentation site with GitHub Pages deployment
(#680)
- add workflow permissions validation for OpenSSF Scorecard compliance
(#759)
- add DT coach return path handoff to task-researcher (#591) (#758)
- add DT subagent handoff workflow instructions (#592) (#757)
- create dt-method-06-deep.instructions.md (#602) (#748)
- create dt-method-05-deep.instructions.md (#747)
- add DT-aware task-implementor context instructions (#755)
- extract embedded PowerShell from workflows into testable scripts
(#738)
- add gitleaks binary-based secret scanning as PR gate (#734)
- add SBOM generation, attestation, and diff tooling to release pipeline
(#730)
- add dt-learning-tutor agent for DT education (#662)
- add DT image prompt generation guidance for Method 5 (#726)
- add DT-aware task-reviewer review context (#714)
- add dt-method-next routing prompt (#713)
- create dt-method-04-deep.instructions.md (#709)
- add Implementation Space exit handoff prompt for DT workflows (#708)
- add Write-CIStepSummary markdown table to Test-SHAStaleness github
output (#660)
- add dt-handoff-solution-space prompt for Solution Spac… (#707)

### 🐛 Bug Fixes

- update prerelease publish to use even/odd convention (#822)
- update sidebar link color to meet WCAG AA contrast requirements (#814)
- harden even/odd versioning against regression and syntax errors (#816)
- replace even/odd versioning with SemVer -rc.N suffixes (#811)
- ensure prerelease label exists before PR creation (#806)
- replace Docusaurus favicons with Microsoft logo (#808)
- add missing subagents and shared instructions to collection manifests
(#804)
- standardize file path conventions for copilot-tracking output (#784)
- enforce project-scoped artifact isolation across DT files (#766)
- add top-level permissions to copilot-setup-steps.yml (#760)
- update broken file directives and markdown links after collection
directory reorg (#743)
- add pre-release companion pipeline with even/odd versioning (#735)
- exclude auto-generated CHANGELOG.md from spell check (#756)
- add job-level permissions to extension-publish.yml (#729)
- resolve handoff dependencies using display names (#727)
- add job-level permissions to validate-version in
extension-publish-prerelease (#731)
- replace parent-directory VS Code settings paths with per-subdirectory
enumeration (#732)

### 📚 Documentation

- add Design Thinking documentation and DT-to-RPI handoff (#789)
- add customization guides for HVE Core artifacts (#772)
- reconcile documentation against implementation (#771)
- document accepted Token-Permissions risks and add
lint:dependency-pinning (#763)
- add Design Thinking section to hve-core-all collection description
(#762)

### ♻️ Refactoring

- move collection scripts from plugins to collections (#728)
- remove duplicate git diff logic in frontmatter validator (#473)

### 🔧 Maintenance

- pre-release 3.1.44 (#819)
- bump basic-ftp from 5.0.5 to 5.2.0 (#780)
- standardize script path references in SKILL.md files (#768)
- bump the github-actions group across 1 directory with 2 updates (#752)

---
*Managed automatically by pre-release workflow.*

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor: Remove duplicate git diff logic in Validate-MarkdownFrontmatter.ps1

4 participants